Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set shortcut listener to modal only if owner is child #13272

Merged
merged 2 commits into from
Mar 11, 2022

Conversation

joheriks
Copy link
Contributor

@joheriks joheriks commented Mar 10, 2022

The previous fix for #13205 (PR #13263) moved all shortcut
registration event sources from UI to the modal component
if one was active; this caused shortcuts owned by components
not under the modal tree to also be moved if they were registered
when a modal component was active, and hence they stopped
working when the modal component tree was removed. This
modifies the previous fix to only change the event source if the
shortcut owner is under the an active modal tree.

@joheriks joheriks requested a review from caalador March 10, 2022 21:53
@github-actions
Copy link

github-actions bot commented Mar 10, 2022

Unit Test Results

   948 files  ±0     948 suites  ±0   36m 46s ⏱️ - 7m 14s
6 161 tests +1  6 112 ✔️ +1  49 💤 ±0  0 ±0 
6 370 runs   - 8  6 318 ✔️  - 5  52 💤  - 3  0 ±0 

Results for commit 68c17cd. ± Comparison against base commit 5fa90d6.

♻️ This comment has been updated with latest results.

The previous fix for #13205 moved all shortcut registration
event source from UI to the modal component if one was
active; this caused shortcuts owned by components not under
the modal tree to also be moved if they were registered when
a modal component was active, and hence they stopped
working when the modal component tree was removed. This
modifies the previous fix to only change the event source if
the shortcut owner is under the an active modal tree.
@joheriks joheriks force-pushed the joheriks/fix-shortcut-on-modal-component-2 branch from 736af20 to 4154d14 Compare March 10, 2022 23:18
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 15 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joheriks joheriks merged commit 4b10fd4 into master Mar 11, 2022
@joheriks joheriks deleted the joheriks/fix-shortcut-on-modal-component-2 branch March 11, 2022 08:56
vaadin-bot pushed a commit that referenced this pull request Mar 11, 2022
The previous fix for #13205 (PR #13263) moved all shortcut
registration event sources from UI to the modal component
if one was active; this caused shortcuts owned by components
not under the modal tree to also be moved if they were registered
when a modal component was active, and hence they stopped
working when the modal component tree was removed. This
modifies the previous fix to only change the event source if the
shortcut owner is under the an active modal tree.
joheriks pushed a commit that referenced this pull request Mar 14, 2022
…13276)

The previous fix for #13205 (PR #13263) moved all shortcut
registration event sources from UI to the modal component
if one was active; this caused shortcuts owned by components
not under the modal tree to also be moved if they were registered
when a modal component was active, and hence they stopped
working when the modal component tree was removed. This
modifies the previous fix to only change the event source if the
shortcut owner is under the an active modal tree.

Co-authored-by: Johannes Eriksson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants